MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in on
#9482
Conversation
…atch group" errors when binary operation has unsorted labels in `on`
There was a problem hiding this comment.
What do you think about having BytesWithLabels (etc) do the sort?
Granted, there are cases where we don't need to, and thus we don't want to add the overhead, so may we have a sort parameter on the function? This means we have to be explicit when using BytesWithLabels and hopefully avoid similar problems in the future
| @@ -98,11 +98,21 @@ eval range from 0 to 24m step 6m left_side - on(env, pod) right_side | |||
| {env="test", pod="a"} -9 -18 -27 | |||
| {env="test", pod="b"} -36 -45 -54 | |||
|
|
|||
| eval range from 0 to 24m step 6m left_side - on(pod, env) right_side | |||
There was a problem hiding this comment.
(nit) should add a comment here on the purpose of these tests and that the load order needs to be preserved
There was a problem hiding this comment.
that the load order needs to be preserved
Not sure I follow this part - what are you referring to by "load order" here?
There was a problem hiding this comment.
the order the series are loaded in (above on line 88) could affect this test right? It's possible they are loaded in a way where the series are already sorted after the on/etc.
There was a problem hiding this comment.
It doesn't matter what order the series are in - the only thing that matters is the order of the label names passed to BytesWithLabels or BytesWithoutLabels.
(and regardless of the order of the series in the load, they'll be retrieved in sorted order due to how chunks streaming works)
|
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9482-to-r308 origin/r308
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x e8e1e13dd9d186954b422406a79eeabbf2cfccaa
# Push it to GitHub
git push --set-upstream origin backport-9482-to-r308
git switch main
# Remove the local backport branch
git branch -D backport-9482-to-r308Then, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9482-to-r309 origin/r309
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x e8e1e13dd9d186954b422406a79eeabbf2cfccaa
# Push it to GitHub
git push --set-upstream origin backport-9482-to-r309
git switch main
# Remove the local backport branch
git branch -D backport-9482-to-r309Then, create a pull request where the |
…atch group" errors when binary operation has unsorted labels in `on` (#9482) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13)
…atch group" errors when binary operation has unsorted labels in `on` (#9482) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13) # Conflicts: # CHANGELOG.md
…atch group" errors when binary operation has unsorted labels in `on` (#9482) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13) # Conflicts: # CHANGELOG.md
…atch group" errors when binary operation has unsorted labels in `on` (#9482) (#9483) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13) Co-authored-by: Charles Korn <[email protected]>
…atch group" errors when binary operation has unsorted labels in `on` (#9482) (#9485) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13) # Conflicts: # CHANGELOG.md
…atch group" errors when binary operation has unsorted labels in `on` (#9482) (#9484) * MQE: fix incorrect query results or "found duplicate series for the match group" errors when binary operation has unsorted labels in `on` * Add changelog entry * Address PR feedback: explain purpose of tests (cherry picked from commit e8e1e13) # Conflicts: # CHANGELOG.md
What this PR does
This PR fixes a bug in MQE's binary operation implementation where query results can be incorrect or incorrectly fail with "found duplicate series for the match group" errors.
labels.Labels.BytesWithLabelsandlabels.Labels.BytesWithoutLabelsexpect the list of label names they receive to be sorted. However, we weren't ensuring this was true whenBytesWithLabelswas called to generate the grouping key for a binary operation withon, so incorrect grouping keys were generated.Which issue(s) this PR fixes or relates to
(none)
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.